Skip to content

fix: remediation code-action follow-up fixes [IDE-2052]#1366

Open
basti-snyk wants to merge 2 commits into
feat/IDE-2052-finding-idfrom
feat/IDE-2052-remediation-followups
Open

fix: remediation code-action follow-up fixes [IDE-2052]#1366
basti-snyk wants to merge 2 commits into
feat/IDE-2052-finding-idfrom
feat/IDE-2052-remediation-followups

Conversation

@basti-snyk

@basti-snyk basti-snyk commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

User description

Summary

Follow-up fixes to the remediation code-action flow, surfaced by review of the fixFolder work (PR #1331). All are correctness/robustness fixes; ~217 lines of production change, the rest tests (outside-in TDD throughout).

  • Restrict remediation quick-fixes to supported products. The filter let fully-fixable Open Source issues through and offered a RemediationAgentQuickFix the agent can't apply. Replaced the negated IaC bypass with an explicit product switch (Code when it has an AI fix; IaC) — OSS/Secrets/Unknown excluded.
  • Propagate the codeAction/resolve context to the provider. The deferred edit ran with context.Background(), so cancelling a resolve didn't stop the remediation. The deferred-edit type now carries a context.Context threaded to Remediate; nil deferred edits are guarded in both the resolve and AI-fix paths.
  • Make the code-action cache concurrency-safe. actionsCache was an unsynchronised map read by codeAction/resolve and written by textDocument/codeAction. Added a mutex covering every access; the entry is read and (via defer) deleted under the lock, with the slow edit run lock-free — so a concurrent retry still finds the entry and the entry is always cleaned up, even on panic.
  • FixFolder rejects a dirty worktree (git status --porcelain --untracked-files=no) so it can't silently fold pre-existing tracked changes into its result, while ignoring untracked artifacts.
  • Gate the snyk.remediationAgent.fixFolder command advertisement on the feature flag (exported RemediationAgentEnabledKey shared with the DI gate). Fixed the filtered-issue debug count; removed the unused NoopProvider.

PR Stack — Merge Order

flowchart LR
    main(["main"])
    PR0["#1330\nremy unit tests"]
    PR1["#1331\nfixFolder + remy integ/smoke + cross-platform"]
    PR2["← YOU ARE HERE\nremediation code-action follow-up fixes"]
    main --> PR0 --> PR1 --> PR2
    style PR2 fill:#ffd700,color:#000
Loading

Depends on: #1331

Note on size

780 changed lines, but 217 are production and ~563 are tests (TDD acceptance/unit + a -race regression test for the cache). Kept as one cohesive bug-fix set rather than fragmenting the review.

Test plan

  • CI green

PR Type

Enhancement, Bug fix


Description

  • Improve remediation code action robustness and concurrency.

  • Enhance context propagation to remediation providers.

  • Add product filtering for remediation actions.

  • Strengthen FixFolder command preconditions and error handling.


Diagram Walkthrough

flowchart LR
  subgraph LSP Client
    Client["Client"]
  end
  subgraph Snyk LS Server
    A[textDocument/codeAction] --> B(CodeActionService.GetCodeActions);
    B --> C{IssuesProvider};
    B --> D[remediationCodeActions];
    D -- Filters --> E(Product Check);
    B --> F(cacheCodeAction);
    F -- Mutex --> G[(actionsCache)];
    H[codeAction/resolve] --> I(CodeActionService.ResolveCodeAction);
    I -- Read/Unlock --> G;
    I -- Pass Context --> J(DeferredEdit);
    I -- Mutex --> G;
    K[FixFolder Command] --> L(Remediator.FixFolder);
    L -- Git Checks --> M{Worktree Status};
    N[Initialize Server] --> O{ExecuteCommandProvider};
    O -- Conditionally adds --> P[RemediationAgentFixFolderCommand];
  end
  Client -- Request --> A;
  Client -- Request --> H;
  Client -- Execute --> K;
Loading

File Walkthrough

Relevant files
Enhancement
8 files
codeaction.go
Make code action cache concurrency-safe and improve remediation logic
+60/-11 
init.go
Export RemediationAgentEnabledKey and use it for feature flag
+6/-3     
codeaction_handlers.go
Pass context to ResolveCodeAction handler                               
+1/-1     
server.go
Conditionally advertise FixFolder command and use exported key
+46/-35 
code_fix.go
Pass context to deferred edits and handle nil deferred edits
+7/-2     
codeaction.go
Update deferred edit signature to accept context.Context 
+7/-3     
code_actions.go
Update OSS quickfix edit callback to accept context.Context
+2/-1     
issues.go
Update CodeAction interface for context in deferred edits
+2/-1     
Tests
11 files
codeaction_test.go
Add tests for cache concurrency and context propagation in code
actions
+186/-5 
export_test.go
Export cache length for testing                                                   
+32/-0   
race_test.go
Add race detector tests for code action cache                       
+145/-0 
remediation_test.go
Test remediation agent context propagation and OSS issue handling
+80/-1   
init_fix_folder_test.go
Use exported RemediationAgentEnabledKey in test                   
+1/-1     
server_smoke_test.go
Use exported RemediationAgentEnabledKey in smoke test       
+1/-1     
server_test.go
Test conditional command advertisement and remediation agent features
+38/-2   
code_fix_test.go
Add test for nil deferred edit in code fix command             
+43/-3   
codeaction_test.go
Update mock deferred edit to accept context.Context           
+3/-2     
remy_fix_folder_test.go
Add tests for FixFolder dirty worktree guard                         
+120/-7 
code_actions_test.go
Update deferred edit calls in OSS tests                                   
+8/-7     
Bug fix
1 files
remy.go
Guard FixFolder against dirty worktrees                                   
+13/-0   
Additional files
2 files
noop.go +0/-32   
noop_test.go +0/-34   

…E-2052]

Follow-up fixes to the remediation code-action path, surfaced by review of
the fixFolder work:

- Restrict remediation quick-fixes to products the agent can handle. The
  filter previously let fixable Open Source issues through and offered a
  RemediationAgentQuickFix the LLM agent cannot apply; replace the negated
  IaC bypass with an explicit product switch (Code when it has an AI fix,
  IaC) and drop everything else.
- Propagate the codeAction/resolve request context to the provider. The
  deferred edit ran with context.Background(), so cancelling a resolve left
  the remediation running; the deferred-edit type now carries a context that
  is threaded through to Remediate, and a nil deferred edit is guarded in
  both the resolve and the AI-fix command paths.
- Make the code-action cache concurrency-safe. actionsCache was an
  unsynchronised map read by codeAction/resolve and written by
  textDocument/codeAction; add a mutex covering every access, take it only to
  read and (via defer) delete the entry, and run the slow edit without the
  lock held so the entry survives a concurrent retry and is always cleaned up,
  even on panic.
- FixFolder now rejects a worktree with uncommitted tracked changes
  (git status --porcelain --untracked-files=no) so it cannot silently fold
  pre-existing edits into its result, while ignoring untracked artifacts.
- Advertise snyk.remediationAgent.fixFolder only when the remediation agent
  is enabled, via an exported RemediationAgentEnabledKey shared with the DI
  gate. Correct the filtered-issue debug count and remove the unused
  NoopProvider.
@basti-snyk

Copy link
Copy Markdown
Contributor Author

/describe

@snyk-io

snyk-io Bot commented Jun 29, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (7a5aa40)

@snyk-pr-review-bot

This comment has been minimized.

@basti-snyk

Copy link
Copy Markdown
Contributor Author

/describe

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Description updated to latest commit (7a5aa40)

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Concurrent Resolve Race 🟡 [minor]

In ResolveCodeAction, actionsCacheMu is released before invoking the slow DeferredEdit. While this prevents blocking other service methods, it allows multiple concurrent goroutines to execute the same remediation process for the same uuid if a client retries the request (e.g., due to a timeout). As noted in the comments, providers must be idempotent/thread-safe per root, but this design causes redundant computation/LLM calls for the exact same finding if the user clicks 'Fix' twice or the IDE retries.

c.actionsCacheMu.Lock()
cached, found := c.actionsCache[key]
c.actionsCacheMu.Unlock()
📚 Repository Context Analyzed

This review considered 56 relevant code sections from 15 files (average relevance: 0.97)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

depends-on-1331 Depends on PR #1331

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant